[RUM-9899]: TracingInterceptor migration #2708

satween · 2025-06-06T09:27:28Z

What does this PR do?

This PR contains major change for tracer migration.
I tried to reduce amount of changes with some tricks with kotlin's typealias but it still huge. Maybe some of you will find it easier to review by commits.

Main changes:

Migration from AndroidTracer to CoreTracer in TracingInterceptor and it descendants.
Refactor and removing duplication logic in unit tests.

Please pay attention to:

Despite the fact that the APIs of CoreTracer and AndroidTracer are similar, some methods were either not supported or supported at a different level of abstraction. In such cases, I implemented the closest and simplest solution and added comments so you could spot them.

Known issues

In some tests StringForgery with regex inside generated 33 length string instead of 32 which was leading to expeption in DDTraceId parsing. Temporary replaced regex "[a-f0-9]{31}". Going to dig deeper in separate ticket.
There is no replacement for GlobalTracer, but TracingInterceptor relies on some globally stored CoreTracer instance. Going to discuss this with the team and provide solution with separate PR.

Review checklist (to be filled by reviewers)

Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
Make sure you discussed the feature or bugfix with the maintaining team in an Issue
Make sure each commit and the PR mention the Issue number (cf the CONTRIBUTING doc)

...-sdk-android-trace-internal/src/main/java/com/datadog/trace/api/interceptor/MutableSpan.java

features/dd-sdk-android-trace-internal/src/main/java/com/datadog/trace/core/PendingTrace.java

...tions/dd-sdk-android-okhttp/src/main/kotlin/com/datadog/android/okhttp/DatadogInterceptor.kt

...k-android-okhttp/src/main/kotlin/com/datadog/android/okhttp/internal/otel/TraceContextExt.kt

...dd-sdk-android-okhttp/src/main/kotlin/com/datadog/android/okhttp/trace/TracingInterceptor.kt

...oid-okhttp/src/test/kotlin/com/datadog/android/okhttp/DatadogInterceptorWithoutTracesTest.kt

codecov-commenter · 2025-06-06T13:54:53Z

Codecov Report

❌ Patch coverage is 65.26946% with 174 lines in your changes missing coverage. Please review.
✅ Project coverage is 70.61%. Comparing base (5957478) to head (1e8df9f).
⚠️ Report is 15 commits behind head on feature/v3.

Files with missing lines	Patch %	Lines
.../com/datadog/opentelemetry/trace/OtelSpanLink.java	0.00%	28 Missing ⚠️
...tadog/android/trace/internal/DatadogSpanAdapter.kt	58.33%	12 Missing and 3 partials ⚠️
...dog/android/trace/internal/DatadogTracerAdapter.kt	64.52%	4 Missing and 7 partials ⚠️
...adog/opentelemetry/trace/OtelExtractedContext.java	0.00%	10 Missing ⚠️
...android/trace/internal/DatadogPropagationHelper.kt	9.09%	10 Missing ⚠️
...atadog/android/trace/internal/DatadogSpanLogger.kt	79.59%	4 Missing and 6 partials ⚠️
...og/android/trace/internal/DatadogTracingToolkit.kt	27.27%	8 Missing ⚠️
...og/trace/core/propagation/TagContextExtractor.java	12.50%	7 Missing ⚠️
...m/datadog/opentelemetry/trace/OtelConventions.java	25.00%	6 Missing ⚠️
...ndroid/trace/opentelemetry/DatadogOpenTelemetry.kt	0.00%	6 Missing ⚠️
... and 32 more

Additional details and impacted files

@@              Coverage Diff               @@
##           feature/v3    #2708      +/-   ##
==============================================
+ Coverage       67.63%   70.61%   +2.98%     
==============================================
  Files             825      797      -28     
  Lines           31026    29124    -1902     
  Branches         5217     4878     -339     
==============================================
- Hits            20983    20564     -419     
+ Misses           8636     7202    -1434     
+ Partials         1407     1358      -49

Files with missing lines	Coverage Δ
...og/android/core/internal/DatadogContextProvider.kt	`98.11% <100.00%> (+0.24%)`	⬆️
...android/log/internal/domain/DatadogLogGenerator.kt	`97.91% <100.00%> (+0.03%)`	⬆️
...ndroid/telemetry/internal/TelemetryEventHandler.kt	`87.82% <100.00%> (+1.45%)`	⬆️
...al/src/main/java/com/datadog/trace/api/Config.java	`62.39% <100.00%> (-0.18%)`	⬇️
...ain/java/com/datadog/trace/api/ConfigDefaults.java	`100.00% <100.00%> (ø)`
...com/datadog/trace/api/interceptor/MutableSpan.java	`0.00% <ø> (ø)`
...trace/bootstrap/instrumentation/api/AgentSpan.java	`0.00% <ø> (ø)`
...race/bootstrap/instrumentation/api/TagContext.java	`53.85% <ø> (ø)`
...og/trace/common/sampling/ForcePrioritySampler.java	`60.00% <ø> (+60.00%)`	⬆️
...va/com/datadog/trace/common/writer/NoOpWriter.java	`66.67% <100.00%> (ø)`
... and 71 more

... and 41 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

...dd-sdk-android-okhttp/src/main/kotlin/com/datadog/android/okhttp/trace/TracingInterceptor.kt

...tions/dd-sdk-android-okhttp/src/main/kotlin/com/datadog/android/okhttp/DatadogInterceptor.kt

features/dd-sdk-android-trace-internal/src/main/java/com/datadog/trace/core/CoreTracer.java

...dd-sdk-android-okhttp/src/main/kotlin/com/datadog/android/okhttp/trace/TracingInterceptor.kt

...k-android-okhttp/src/main/kotlin/com/datadog/android/okhttp/internal/otel/TraceContextExt.kt

...-sdk-android-trace-internal/src/main/java/com/datadog/trace/api/interceptor/MutableSpan.java

features/dd-sdk-android-trace-internal/src/main/java/com/datadog/trace/core/DDSpan.java

features/dd-sdk-android-trace-internal/src/main/java/com/datadog/trace/core/PendingTrace.java

mariusc83

LGTM in general. We're missing the part where we are able to inject our own tracer into the DatadogInterceptor#builder, see the Slack thread but I guess this can still be added into this PR.

...ions/dd-sdk-android-okhttp/src/main/kotlin/com/datadog/android/okhttp/trace/OtelMigration.kt

dd-sdk-android-internal/src/main/java/com/datadog/android/internal/utils/CastExt.kt

@nikita

the moment of this commit this new API was never used, so by composite decision with @nikita and @marius we decided to use EXTERNAL_OVERRIDE as sampling mechanism here.

… resolution during one of last commit squashing procedure

…xtension implementations

…eTracer and related classes for customers. The only publicity available ways for tracing feature usage: - okhttp instrumentation - manual instrumentation with Otel so it's okay that TracerContext is the only way to set the parent span (otherwise AgentSpan will be exposed)

…ods flow

aleksandr-gringauz · 2025-08-08T12:47:20Z

lgtm

but better wait also for @mariusc83 approval

features/dd-sdk-android-trace/build.gradle.kts

features/dd-sdk-android-trace-api/build.gradle.kts

...trace-internal/src/main/java/com/datadog/trace/bootstrap/instrumentation/api/TagContext.java

...s/dd-sdk-android-trace-internal/src/main/java/com/datadog/trace/api/config/TracerConfig.java

sample/kotlin/src/main/kotlin/com/datadog/android/sample/SampleApplication.kt

… to meta, reverting _dd object back to log

ci/pipelines/default-pipeline.yml

detekt_custom.yml

0xnm · 2025-08-14T15:15:08Z

features/dd-sdk-android-logs/api/apiSurface

@@ -35,17 +35,17 @@ data class com.datadog.android.log.LogsConfiguration
    fun setEventMapper(com.datadog.android.event.EventMapper<com.datadog.android.log.model.LogEvent>): Builder
    fun build(): LogsConfiguration
 data class com.datadog.android.log.model.LogEvent
-  constructor(Device, Os, Status, kotlin.String, kotlin.String, kotlin.String, Logger, Usr? = null, Account? = null, Network? = null, Error? = null, kotlin.String? = null, kotlin.String, kotlin.collections.MutableMap<kotlin.String, kotlin.Any?> = mutableMapOf())
+  constructor(LogEventDevice, Os, Status, kotlin.String, kotlin.String, kotlin.String, Logger, Dd, Usr? = null, Account? = null, Network? = null, Error? = null, kotlin.String? = null, kotlin.String, kotlin.collections.MutableMap<kotlin.String, kotlin.Any?> = mutableMapOf())


it seems these changes should already be in v3, is develop only merged in the tracing branch?

This change appears because we have to revert os and device back to meta in this commit. The change was made in this branch, but not in separate ticket to speed up next dogfooding interation

...id-trace-api/src/main/kotlin/com/datadog/android/trace/api/propagation/DatadogPropagation.kt

...s/dd-sdk-android-trace-api/src/main/kotlin/com/datadog/android/trace/api/span/DatadogSpan.kt

0xnm · 2025-08-14T16:25:57Z

instrumented/integration/proguard-rules.pro

-# Required because we need access to GlobalTracer isRegistered property to reset it through reflection
-keepnames class io.opentracing.util.GlobalTracer {
-    private boolean isRegistered;
+# Required because we need access to GlobalDatadogTracer getOrNull method to reset it through reflection


this is already declared in the trace module, so I guess re-declaration is not needed?

Not tested yet, gonna remove in separate ticket RUM-11362 (after we make a decision regarding publicitly available DatadogTracer)

What I mean here is that it is already declared in the Proguard rules file of the Tracing module and so will be added from there.

...dd-sdk-android-okhttp/src/main/kotlin/com/datadog/android/okhttp/trace/TracingInterceptor.kt

...khttp/src/test/kotlin/com/datadog/android/okhttp/internal/utils/forge/TraceContextFactory.kt

...bility/single-fit/okhttp/src/test/kotlin/com/datadog/android/okhttp/HeadBasedSamplingTest.kt

fun activateSpan(span: DatadogSpan, asyncPropagating: Boolean): DatadogScope? internal

0xnm · 2025-08-26T08:08:52Z

...s/dd-sdk-android-trace-api/src/main/kotlin/com/datadog/android/trace/api/span/DatadogSpan.kt

+    /**
+     * Represents the duration of a span in nanoseconds.
+     */
+    val durationNano: Long
+
+    /**
+     * Represents the start time of the span in nanoseconds.
+     */
+    val startTimeNanos: Long


minor: it is better to have consistent naming, so it should be durationNanos

Gonna address this in next PR if possible

0xnm · 2025-08-26T08:11:11Z

features/dd-sdk-android-trace/api/apiSurface

+  fun get(): com.datadog.android.trace.api.tracer.DatadogTracer
+  fun getOrNull(): com.datadog.android.trace.api.tracer.DatadogTracer?


do we need both? maybe we should leave only one of them?

Gonna address this in next PR if possible